-
Notifications
You must be signed in to change notification settings - Fork 23.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
route53_facts: add check mode support #56900
route53_facts: add check mode support #56900
Conversation
@zyv, just so you are aware we have a dedicated Working Group for aws. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shipit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Needs a changelog fragment, though.
Hi @felixfontein, sorry, we didn't know about fragments, must be something new! Maybe worth teaching the bot or CI to nag submitters about it. Anyways, just added one, hopefully correctly; thank you for taking your time to check this PR out! |
@@ -0,0 +1,2 @@ | |||
minor_changes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this should be a bugfix, not a new feature. The module should have advertised that it supports check mode since the beginning.
@zyv changelog fragments were introduced a year ago (just looked it up, the first announcement came on June 5th 2018 :) ). I'm not sure since when they are mandatory; the problem with CI/bot complaining about them is that they are not necessary for everything (a lot of doc changes don't require them, for example), and I'm not aware of current discussions/ideas of whether / how to enforce them. |
Co-Authored-By: Felix Fontein <felix@fontein.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shipit
@zyv do you want to create a backport to stable-2.8? Or anyone else wants to do it? |
* route53_facts: add check mode support * route53_facts: add changelog fragment mentioning check mode support * route53_facts: alter changelog fragment type from `minor_changes` to `bugfixes` * Update changelogs/fragments/56900-route53-facts-check-mode.yaml Co-Authored-By: Felix Fontein <felix@fontein.de>
SUMMARY
This PR adds check mode support for
route53_facts
.ISSUE TYPE
COMPONENT NAME
route53_facts